[ECS-Plugin] Implement ECS_ SYNC stage by armistcxy · Pull Request #6559 · pipe-cd/pipecd

armistcxy · 2026-03-06T08:31:27Z

What this PR does:
Implementing ECS_SYNC stage. The quick sync and sync option will use this stage by default

Why we need it:

Which issue(s) this PR fixes: Part of #6443

Fixes #

Does this PR introduce a user-facing change?:

How are users affected by this change:
Is this breaking change:
How to migrate (if breaking change):

Signed-off-by: Hoang Ngo <adlehoang118@gmail.com>

when recreate is enable, stop the current running tasks (i.e. set desired #tasks of service to 0), then rollout new taskset and set the number of desired #tasks back to original value Signed-off-by: Hoang Ngo <adlehoang118@gmail.com>

Signed-off-by: Hoang Ngo <adlehoang118@gmail.com>

armistcxy · 2026-03-06T08:38:31Z

There some works still needed to do before merging, I 've just created this PR so you (@khanhtc1202) can have a quick glance and give early feedbacks ^ ^

To make it easy when reviewing PR, you can refer to these resources:

Some changes have been made (different from v0)

No longer check PipCD managed tags for service: Change
Check service's health status before update service: Check
Remove redundant slice copy in client.ListTags

func (c *client) ListTags(ctx context.Context, resourceArn string) ([]types.Tag, error) {
	input := &ecs.ListTagsForResourceInput{
		ResourceArn: aws.String(resourceArn),
	}

	output, err := c.ecsClient.ListTagsForResource(ctx, input)
	if err != nil {
		return nil, err
	}

	tags := make([]types.Tag, 0, len(output.Tags))
	for _, t := range output.Tags {
		tags = append(tags, types.Tag{
			Key:   t.Key,
			Value: t.Value,
		})
	}
	return tags, nil
}

TODO (in works):

Figure out how to implement test for sync stage -> use mock for testing (logic-focus only, not real integrated) add unit test for sync function
Figure out how to support other properties/fields for tasks and services (currently only supports v0-compatible props)

armistcxy · 2026-03-06T08:56:48Z

About the testing part, I've already made something like this to allow real integration test. But this is not safe due to potential waste if not handle cleaning up resources properly after test, maybe mocking will be better here

	var (
		clusterARN   = os.Getenv("ECS_CLUSTER_ARN")
		subnetIDsStr = os.Getenv("SUBNET_IDS")
		sgIDsStr     = os.Getenv("SECURITY_GROUP_IDS")

		subnetIDs []string
		sgIDs     []string
	)

	if clusterARN == "" {
		t.Skip("ECS_CLUSTER_ARN is not set, skipping ECS sync stage test")
	}
	if subnetIDsStr == "" {
		t.Skip("SUBNET_IDS is not set, skipping ECS sync stage test")
	}
	for _, id := range strings.Split(subnetIDsStr, ",") {
		subnetIDs = append(subnetIDs, strings.TrimSpace(id))
	}
	if len(subnetIDs) == 0 {
		t.Skip("SUBNET_IDS is empty, skipping ECS sync stage test")
	}
	if sgIDsStr == "" {
		t.Skip("SECURITY_GROUP_IDS is not set, skipping ECS sync stage test")
	}

khanhtc1202 · 2026-03-09T01:10:35Z

For DetermineVersion function, I think you can ignore it in this PR, we can implement it by a separated PR

khanhtc1202 · 2026-03-09T01:13:20Z

Also, your commits had shown as Unverified currently, could you check this docs and update Github setting to fix that
https://docs.github.com/en/authentication/managing-commit-signature-verification/displaying-verification-statuses-for-all-of-your-commits
Probably, this is caused by Github SSH Key is not being used

khanhtc1202 · 2026-03-09T01:16:33Z

pkg/app/pipedv1/plugin/ecs/deployment/sync.go

+	// Add PipeCD-managed tags
+	serviceDef.Tags = append(serviceDef.Tags,
+		provider.MakeTags(map[string]string{
+			provider.LabelManagedBy:   provider.ManagedByECSPlugin,
+			provider.LabelPiped:       input.Request.Deployment.PipedID,
+			provider.LabelApplication: input.Request.Deployment.ApplicationID,
+			provider.LabelCommitHash:  input.Request.TargetDeploymentSource.CommitHash,
+		})...,
+	)


How about to move in inside LoadServiceDefinition function? Since we may need to do the same when implement rollout stage.

I think LoadServiecDefinition purpose is parsing manifest into types.Service, adding tags here I just feel not the right place to do that

IMO, adding tags explicitly in execute stage function will avoid confusion for the maintain work in the future

Also, LoadServiceDefinition current input consists of 2 params: appDir, serviceDefinition string, if moving adding tags to this function, we need to add pipeId, applicationId, commitHash, which will make the function bloat and drift away from its function

Okay, let re-check the service tags value of service object fetched from API when we update service. If it contains the tags as well, we can safely move this tag assign logic to load service definition, ensure service object in data flow will always have the tags. 👍
For now, let's keep the implementation as your 👍

The service returned from UpdateService API doesn't contain tags but all the stage use applyServiceDefinition and inside that function the tags are re assigned

// Re-assign tags to service object because UpdateService API doesn't return tags. service.Tags = serviceDefinition.Tags

And my bad, the v0 loadServiceDefinition already add the tags inside

func loadServiceDefinition(in *executor.Input, serviceDefinitionFile string, ds *deploysource.DeploySource) (types.Service, bool) { in.LogPersister.Infof("Loading service manifest at commit %s", ds.Revision) serviceDefinition, err := provider.LoadServiceDefinition(ds.AppDir, serviceDefinitionFile) if err != nil { in.LogPersister.Errorf("Failed to load ECS service definition (%v)", err) return types.Service{}, false } serviceDefinition.Tags = append( serviceDefinition.Tags, provider.MakeTags(map[string]string{ provider.LabelManagedBy: provider.ManagedByPiped, provider.LabelPiped: in.PipedConfig.PipedID, provider.LabelApplication: in.Deployment.ApplicationId, provider.LabelCommitHash: in.Deployment.CommitHash(), })..., ) in.LogPersister.Infof("Successfully loaded the ECS service definition at commit %s", ds.Revision) return serviceDefinition, true }

Update:

// LoadServiceDefinition returns Service object from a given service definition file. func LoadServiceDefinition(appDir, serviceDefinition string, input *sdk.ExecuteStageInput[config.ECSApplicationSpec]) (types.Service, error) { path := filepath.Join(appDir, serviceDefinition) service, err := loadServiceDefinition(path) if err != nil { return types.Service{}, err } // Add PipeCD-managed tags service.Tags = append(service.Tags, MakeTags(map[string]string{ LabelManagedBy: ManagedByECSPlugin, LabelPiped: input.Request.Deployment.PipedID, LabelApplication: input.Request.Deployment.ApplicationID, LabelCommitHash: input.Request.TargetDeploymentSource.CommitHash, })..., ) return service, nil }

Resolved with chore: move add pipecd-related tags to load service function

pkg/app/pipedv1/plugin/ecs/deployment/sync.go

pkg/app/pipedv1/plugin/ecs/config/sync.go

Signed-off-by: Hoang Ngo <adlehoang118@gmail.com>

khanhtc1202

LGTM 💯

armistcxy added 6 commits March 6, 2026 15:26

feat: implement load, parse task definition and service from file

c7ffdb6

Signed-off-by: Hoang Ngo <adlehoang118@gmail.com>

feat: implement pool of wrapper ecs client

2ee8be5

Signed-off-by: Hoang Ngo <adlehoang118@gmail.com>

feat: implement ECS_SYNC stage (basic flow)

c755875

Signed-off-by: Hoang Ngo <adlehoang118@gmail.com>

handle flag recreate in sync stage

ecfe835

when recreate is enable, stop the current running tasks (i.e. set desired #tasks of service to 0), then rollout new taskset and set the number of desired #tasks back to original value Signed-off-by: Hoang Ngo <adlehoang118@gmail.com>

handle maintain desired tags for ecs service

0b0fb59

Signed-off-by: Hoang Ngo <adlehoang118@gmail.com>

add pipecd managed tags for ecs service

a30008e

Signed-off-by: Hoang Ngo <adlehoang118@gmail.com>

armistcxy requested a review from a team as a code owner March 6, 2026 08:31

armistcxy requested review from Warashi, ffjlabo and khanhtc1202 March 6, 2026 08:31

github-actions bot added area/go area/pipedv1 labels Mar 6, 2026

armistcxy self-assigned this Mar 6, 2026

armistcxy added the do-not-merge/wip label Mar 6, 2026

armistcxy mentioned this pull request Mar 6, 2026

[LFX Mentorship T-1 2026] Implement Amazon ECS Plugin for pipedv1 #6443

Open

khanhtc1202 reviewed Mar 9, 2026

View reviewed changes

pkg/app/pipedv1/plugin/ecs/deployment/sync.go Outdated Show resolved Hide resolved

khanhtc1202 reviewed Mar 9, 2026

View reviewed changes

pkg/app/pipedv1/plugin/ecs/config/sync.go Show resolved Hide resolved

armistcxy added 2 commits March 9, 2026 10:42

chore: add expected behavior of configuration

b05fcb5

Signed-off-by: Hoang Ngo <adlehoang118@gmail.com>

chore: move tags to nearest used place

983f35f

Signed-off-by: Hoang Ngo <adlehoang118@gmail.com>

armistcxy mentioned this pull request Mar 9, 2026

[ECS-Plugin] Implement ECS_ROLLBACK stage #6573

Merged

add unit test for sync function

7b35462

Signed-off-by: Hoang Ngo <adlehoang118@gmail.com>

armistcxy force-pushed the ecs-plugin/sync branch from 5f46f2e to 7b35462 Compare March 9, 2026 09:52

chore: move add pipecd-related tags to load service function

49dfb32

Signed-off-by: Hoang Ngo <adlehoang118@gmail.com>

armistcxy removed the do-not-merge/wip label Mar 10, 2026

khanhtc1202 approved these changes Mar 11, 2026

View reviewed changes

khanhtc1202 merged commit 99fede9 into pipe-cd:master Mar 11, 2026
45 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ECS-Plugin] Implement ECS_ SYNC stage#6559

[ECS-Plugin] Implement ECS_ SYNC stage#6559
khanhtc1202 merged 10 commits intopipe-cd:masterfrom
armistcxy:ecs-plugin/sync

armistcxy commented Mar 6, 2026

Uh oh!

armistcxy commented Mar 6, 2026 •

edited

Loading

Uh oh!

armistcxy commented Mar 6, 2026 •

edited

Loading

Uh oh!

khanhtc1202 commented Mar 9, 2026

Uh oh!

khanhtc1202 commented Mar 9, 2026

Uh oh!

khanhtc1202 Mar 9, 2026

Uh oh!

armistcxy Mar 9, 2026

Uh oh!

armistcxy Mar 9, 2026

Uh oh!

khanhtc1202 Mar 10, 2026

Uh oh!

armistcxy Mar 10, 2026

Uh oh!

armistcxy Mar 10, 2026

Uh oh!

Uh oh!

Uh oh!

khanhtc1202 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

armistcxy commented Mar 6, 2026

Uh oh!

armistcxy commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

armistcxy commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

khanhtc1202 commented Mar 9, 2026

Uh oh!

khanhtc1202 commented Mar 9, 2026

Uh oh!

khanhtc1202 Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

armistcxy Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

armistcxy Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

khanhtc1202 Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

armistcxy Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

armistcxy Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

khanhtc1202 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

armistcxy commented Mar 6, 2026 •

edited

Loading

armistcxy commented Mar 6, 2026 •

edited

Loading